Support devirtualization for virtual methods that require runtime lookups by hez2010 · Pull Request #129806 · dotnet/runtime

hez2010 · 2026-06-24T16:44:25Z

Add support for devirtualizing virtual methods that require a runtime lookup. In this case we can't avoid the expensive runtime lookup, but at least the callee now can be devirtualized and inlined, so that they can benefit from the downstream optimizations like escape analysis, struct promotion and etc.

When we see an inexact instantiating stub on an exact class, we compute a runtime lookup against the shared generic token of the virtual method.

public interface IProcessor
{
    object? Process<T>(T value);
}

public class PrintProcessor : IProcessor
{
    public object? Process<T>(T value)
    {
        return value;
    }
}

public static class Program
{
    public static void Main()
    {
        Console.WriteLine(Call("hello"));
    }

    [MethodImpl(MethodImplOptions.NoInlining)]
    static object? Call<T>(T value) where T : class
    {
        IProcessor processor = new PrintProcessor();
        return processor.Process(value);
    }
}

Codegen for Call:

Before:

G_M47477_IG01:  ;; offset=0x0000
       push     rdi
       push     rsi
       push     rbx
       sub      rsp, 48
       mov      qword ptr [rsp+0x28], rcx
       mov      rbx, rcx
       mov      rsi, rdx
                                                ;; size=18 bbWeight=1 PerfScore 4.75
G_M47477_IG02:  ;; offset=0x0012
       mov      rcx, 0x7FF9258BB130      ; PrintProcessor
       call     CORINFO_HELP_NEWSFAST
       mov      rdi, rax
       mov      rcx, qword ptr [rbx+0x48]
       mov      r8, qword ptr [rcx+0x10]
       test     r8, r8
       je       SHORT G_M47477_IG05
                                                ;; size=31 bbWeight=1 PerfScore 6.75
G_M47477_IG03:  ;; offset=0x0031
       mov      rcx, rdi
       mov      rdx, 0x7FF9258BAF08      ; IProcessor
       call     CORINFO_HELP_VIRTUAL_FUNC_PTR
       mov      rcx, rdi
       mov      rdx, rsi
       call     rax
       nop
                                                ;; size=27 bbWeight=1 PerfScore 5.25
G_M47477_IG04:  ;; offset=0x004C
       add      rsp, 48
       pop      rbx
       pop      rsi
       pop      rdi
       ret
                                                ;; size=8 bbWeight=1 PerfScore 2.75
G_M47477_IG05:  ;; offset=0x0054
       mov      rcx, rbx
       mov      rdx, 0x7FF9258E0DE8      ; global ptr
       call     CORINFO_HELP_RUNTIMEHANDLE_METHOD
       mov      r8, rax
       jmp      SHORT G_M47477_IG03
                                                ;; size=23 bbWeight=0.20 PerfScore 0.75

After:

G_M47477_IG01:  ;; offset=0x0000
       sub      rsp, 40
       mov      qword ptr [rsp+0x20], rcx
                                                ;; size=9 bbWeight=1 PerfScore 1.25
G_M47477_IG02:  ;; offset=0x0009
       mov      rax, qword ptr [rcx+0x48]
       cmp      qword ptr [rax+0x08], 24
       jle      SHORT G_M47477_IG03
                                                ;; size=11 bbWeight=1 PerfScore 5.00
G_M47477_IG03:  ;; offset=0x0014
       mov      rax, rdx
                                                ;; size=3 bbWeight=1 PerfScore 0.25
G_M47477_IG04:  ;; offset=0x0017
       add      rsp, 40
       ret
                                                ;; size=5 bbWeight=1 PerfScore 1.25

Contributes to #112596

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

hez2010 · 2026-06-24T16:47:18Z

@MihuBot -nuget

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

hez2010 · 2026-06-25T04:15:19Z

@MihuBot -nuget -dependsOn 128702

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

azure-pipelines · 2026-07-01T15:17:03Z

Azure Pipelines successfully started running 3 pipeline(s).

dotnet-policy-service · 2026-07-01T15:17:27Z

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

Copilot

Pull request overview

Copilot reviewed 19 out of 19 changed files in this pull request and generated 2 comments.

hez2010 · 2026-07-03T02:44:53Z

I think I have addressed the late devirt context mismatch issue and added tests for this scenario.
@jakobbotsch can you trigger another pri1 run?

Copilot

Pull request overview

Copilot reviewed 19 out of 19 changed files in this pull request and generated 1 comment.

                if (originalImpl.IsRuntimeDeterminedExactMethod || originalImpl.IsSharedByGenericInstantiations)
                {
-                    // TODO: Support for runtime lookup
+                    if (info->pResolvedTokenVirtualMethod == null || unboxingStub)
+                    {
+                        info->detail = CORINFO_DEVIRTUALIZATION_DETAIL.CORINFO_DEVIRTUALIZATION_FAILED_CANON;
+                        return false;
+                    }


jakobbotsch · 2026-07-03T08:17:05Z

/azp run runtime-coreclr outerloop, runtime-coreclr jitstress, runtime-coreclr libraries-jitstress

azure-pipelines · 2026-07-03T08:17:30Z

Azure Pipelines successfully started running 3 pipeline(s).

hez2010 · 2026-07-03T14:08:44Z

All failures are known issues and unrelated. PTAL.

jakobbotsch · 2026-07-13T10:42:31Z

    CORINFO_METHOD_HANDLE  methodHnd;
    CORINFO_CONTEXT_HANDLE exactContextHnd;
    ILLocation             ilLocation;
+    CORINFO_RESOLVED_TOKEN resolvedToken;


Resolved tokens are large structures. Does this show up on memory use?

I didn't measure it yet, but it is necessary for computing the runtime lookup. I can't think of a better way than disabling late devirtualization for this if we don't want to store a resolved token.

jakobbotsch · 2026-07-13T10:45:25Z

+        // If we don't have the right context, try recover it from the tree.
+        //
+        if (!isCurrentContext)
+        {
+            CallArg* runtimeMethodHandleArg = nullptr;
+            if ((call->gtControlExpr != nullptr) && call->gtControlExpr->OperIs(GT_CALL))
+            {
+                runtimeMethodHandleArg =
+                    call->gtControlExpr->AsCall()->gtArgs.FindWellKnownArg(WellKnownArg::RuntimeMethodHandle);
+            }
+
+            if (runtimeMethodHandleArg != nullptr && runtimeMethodHandleArg->GetNode()->OperIs(GT_RUNTIMELOOKUP))
+            {
+                if (runtimeMethodHandleArg->GetNode()->AsRuntimeLookup()->Lookup()->OperIs(GT_CALL))
+                {
+                    GenTreeCall* const helperCall =
+                        runtimeMethodHandleArg->GetNode()->AsRuntimeLookup()->Lookup()->AsCall();
+
+                    if (helperCall->IsHelperCall(CORINFO_HELP_RUNTIMEHANDLE_METHOD) ||
+                        helperCall->IsHelperCall(CORINFO_HELP_RUNTIMEHANDLE_CLASS))
+                    {
+                        runtimeLookupContext = helperCall->gtArgs.GetArgByIndex(0)->GetNode();
+                    }
+                }
+            }
+
+            if (runtimeLookupContext == nullptr)
+            {
+                JITDUMP("Late devirt needs a runtime lookup context cannot be figured out. Bail out.\n");
+                return;
+            }
+        }


Why can this not be done where it is needed?

Trying to smuggle IR through across phases without actually adding it does not scale well. I do not think this is a workable way of doing this.

Ah, dcInfo is not the info we save. Ok, then it looks better.

Trying to smuggle IR through across phases without actually adding it does not scale well.

Alternatively we will need to add something to GenTreeCall to carry the original lookup context tree.

github-actions · 2026-07-19T09:38:50Z

Workflow state for the Holistic Review Orchestrator.

{
  "version": 5,
  "last_dispatched_commit": "a9780b4c86ad8a2f839ef4abad70f63e3737b6e3",
  "last_dispatched_base_ref": "main",
  "last_dispatched_base_sha": "de625a927aa4e6f70cda5eaf247cf54c225d158a",
  "last_reviewed_commit": "a9780b4c86ad8a2f839ef4abad70f63e3737b6e3",
  "last_reviewed_base_ref": "main",
  "last_reviewed_base_sha": "de625a927aa4e6f70cda5eaf247cf54c225d158a",
  "last_recorded_worker_run_id": "29681945714",
  "review_attempt_commit": "",
  "review_attempt_base_ref": "",
  "review_attempt_count": 0,
  "max_review_attempts": 5,
  "review_history_format": "holistic-review-disclosure-v1",
  "review_history": [
    {
      "commit": "a9780b4c86ad8a2f839ef4abad70f63e3737b6e3",
      "review_id": 4730548930
    }
  ]
}

github-actions

Holistic Review

Motivation: The problem is real and worthwhile. Today the JIT bails out of devirtualizing a shared generic virtual method whose instantiation argument requires a runtime lookup (CORINFO_DEVIRTUALIZATION_FAILED_CANON). This PR lets such calls devirtualize (still paying the runtime lookup) so the callee can be inlined and benefit from escape analysis, struct promotion, etc. The demonstrated codegen improvement is significant and it contributes to #112596.

Approach: Reasonable and consistent with existing infrastructure. It adds a new DevirtualizedMethodDescSlot dictionary entry kind wired through the VM, the R2R/NativeAOT ComputeRuntimeLookupForSharedGenericToken, and SuperPMI, threads a callerMethod input into resolveVirtualMethod, and propagates the runtime-lookup context tree from dcInfo into getLookupTree/getRuntimeLookupTree. The JIT-EE GUID is correctly bumped. NativeAOT is intentionally left as a TODO. The trickiest part is late devirtualization recovering the correct generic context from the existing call IR; the author added a LateDevirtualizationInfo.resolvedToken and targeted tests for the immediate/late/late-root context cases.

Summary: ⚠️ Needs Human Review. The change is well-constructed and the new-success paths look internally consistent (all three CORINFO_DEVIRTUALIZATION_INFO construction sites now initialize callerMethod, and the shared-MT vs shared-method cases bail vs. runtime-lookup as intended). However, two areas warrant maintainer judgment: (1) the impRuntimeLookupToTree behavioral change is broader than the devirt feature and touches all optimized testForNull runtime lookups (see inline); and (2) jakobbotsch already raised concerns about smuggling the lookup-context IR across phases via dcInfo and about the memory cost of storing a full CORINFO_RESOLVED_TOKEN in LateDevirtualizationInfo. These are design/CQ tradeoffs best confirmed by the JIT area owners plus SPMI diffs, not blockers I can resolve from the diff alone.

Detailed Findings

⚠️ Broadened behavior in `impRuntimeLookupToTree` — un-spilled helper call in optimized code

The opts.OptimizationEnabled() branch now returns the raw helper GT_CALL instead of always spilling to a temp. This applies to every testForNull runtime lookup in optimized code, well beyond the devirtualization scenario the PR targets. Flagged inline on src/coreclr/jit/importer.cpp; please validate with a full SPMI/CQ run that no unrelated regressions or duplicated side-effecting calls result, and consider scoping it.

✅ `callerMethod` initialization is complete

The new required CORINFO_DEVIRTUALIZATION_INFO.callerMethod input is set at all three JIT construction sites (importercalls.cpp lines 8097, 8183, 9296) from call->gtInlineContext->GetCallee(). An earlier Copilot comment worried about uninitialized garbage in the GDV exact-classes loop; that concern does not apply to the current head — line 8097 initializes it. The VM/R2R runtime-lookup paths that dereference callerMethod are only reached after pResolvedTokenVirtualMethod != nullptr, and in the JIT that token is only non-null on the direct-devirt path (GDV likely/exact still pass nullptr), so GetMethod/HandleToObject on callerMethod is not exercised with an unset handle today. Worth a maintainer double-check that no external JIT-EE consumer leaves callerMethod unset.

✅ Shared-MT vs shared-method distinction

The VM (resolveVirtualMethodHelper) and managed (CorInfoImpl.cs) paths correctly bail with FAILED_CANON when a shared MethodTable instantiation is required (cannot be recovered even as a runtime lookup) and only take the new DevirtualizedMethodDescSlot runtime-lookup path when the method instantiation is canonical. The isUnboxingStubOfInstantiatingStub case was split accordingly, and the unboxed-entry lookup now unwraps an instantiating stub (jitinterface.cpp 8979).

✅ Late-devirtualization context recovery + tests

The RuntimeLookupInlining test exercises immediate inlining, late devirtualization of an inlinee call (context mismatch requiring recovery of the runtime-lookup context from the RuntimeMethodHandle arg), and the late-root case. The bail-out when the context tree cannot be recovered (impDevirtualizeCall) is a safe conservative fallback. This addresses the JIT guidance's expectation of targeted tests for an observable behavior change.

💡 Style nit in `jitinterface.cpp`

The new single-line braceless if at line 8979 is inconsistent with the file's bracing style; flagged inline (non-blocking).

💡 Open design questions from maintainer review

jakobbotsch questioned passing the lookup context IR through dcInfo across phases and the memory footprint of embedding a full CORINFO_RESOLVED_TOKEN in LateDevirtualizationInfo (resolved tokens are large). The author acknowledged both and noted the alternative would be adding a field to GenTreeCall. These remain unresolved design tradeoffs for the area owners to sign off on.

Note

This review was generated by this repository's Holistic Review agentic workflow to complement the built-in Copilot review.

Generated by Holistic Review · 190.1 AIC · ⌖ 11.5 AIC · ⊞ 10K

Copilot AI review requested due to automatic review settings June 24, 2026 16:44

Copilot AI reviewed Jun 24, 2026

github-actions Bot added the area-crossgen2-coreclr only use for closed issues label Jun 24, 2026

dotnet-policy-service Bot added the community-contribution Indicates that the PR has been added by a community member label Jun 24, 2026

MihuBot mentioned this pull request Jun 24, 2026

[JitDiff X64] [hez2010] Support devirtualization for virtual methods that re ... MihuBot/runtime-utils#2047

Open

hez2010 mentioned this pull request Jun 24, 2026

JIT: Devirtualization for generic virtual methods #112596

Open

14 tasks

Copilot AI review requested due to automatic review settings June 24, 2026 23:43

Copilot AI reviewed Jun 24, 2026

MihuBot mentioned this pull request Jun 25, 2026

[JitDiff X64] [hez2010] Support devirtualization for virtual methods that re ... MihuBot/runtime-utils#2050

Open

Copilot AI review requested due to automatic review settings June 29, 2026 12:51

Copilot AI reviewed Jun 29, 2026

hez2010 added 5 commits June 29, 2026 22:05

CoreCLR support

2723cdd

R2R

9ce2f81

Various fixes

2f787b6

JIT format

071523a

Format

309d8fd

hez2010 force-pushed the devirt-runtime-lookups branch from 5497d14 to 309d8fd Compare June 29, 2026 13:07

hez2010 added 2 commits July 1, 2026 20:10

Merge branch 'main' into devirt-runtime-lookups

7721087

Remove unnecessary unboxing stub handling

0dc166e

Copilot AI review requested due to automatic review settings July 1, 2026 11:33

Copilot started reviewing on behalf of hez2010 July 1, 2026 11:34 View session

Support runtime lookup together with unboxing stub

7de7f0c

Copilot AI reviewed Jul 1, 2026

View reviewed changes

Comment thread src/coreclr/jit/importercalls.cpp

Copilot AI review requested due to automatic review settings July 1, 2026 11:42

Copilot started reviewing on behalf of hez2010 July 1, 2026 11:42 View session

hez2010 added 3 commits July 3, 2026 00:33

Find the lookup context from IR in late devirt

218669d

Add tests

e93af8c

Merge branch 'main' into devirt-runtime-lookups

ab94545

Copilot AI review requested due to automatic review settings July 2, 2026 16:28

Copilot started reviewing on behalf of hez2010 July 2, 2026 16:28 View session

Copilot AI reviewed Jul 2, 2026

View reviewed changes

Comment thread src/coreclr/jit/importercalls.cpp

Comment thread src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp

Address feedbacks

fb42135

Remove redundant clone as the old context will be thrown away

a9780b4

Copilot AI review requested due to automatic review settings July 3, 2026 03:24

Copilot started reviewing on behalf of hez2010 July 3, 2026 03:25 View session

Copilot AI reviewed Jul 3, 2026

View reviewed changes

build-analysis Bot mentioned this pull request Jul 3, 2026

ILC failing with exit code 139 for risc-v build #130145

Closed

This was referenced Jul 3, 2026

[ci-scan] Test failure: System.Threading.Tests.MutexTests.CrossProcess_NamedMutex_ConcurrentCreateOrOpen #130125

Closed

[ci-scan] Test failure: System.Transactions.Tests.AsyncTransactionScopeTests.AsyncTSTest #130126

Closed

hez2010 mentioned this pull request Jul 6, 2026

JIT: Centralize WellKnownArg definitions #130252

Merged

JulieLeeMSFT requested a review from jakobbotsch July 6, 2026 22:25

JulieLeeMSFT assigned hez2010 Jul 6, 2026

jakobbotsch reviewed Jul 13, 2026

View reviewed changes

github-actions Bot reviewed Jul 19, 2026

View reviewed changes

Comment thread src/coreclr/jit/importer.cpp

Comment thread src/coreclr/vm/jitinterface.cpp

Uh oh!

Conversation

hez2010 commented Jun 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

hez2010 commented Jun 24, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

hez2010 commented Jun 25, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

azure-pipelines Bot commented Jul 1, 2026

Uh oh!

dotnet-policy-service Bot commented Jul 1, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

hez2010 commented Jul 3, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

jakobbotsch commented Jul 3, 2026

Uh oh!

azure-pipelines Bot commented Jul 3, 2026

Uh oh!

hez2010 commented Jul 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jakobbotsch Jul 13, 2026

Choose a reason for hiding this comment

Uh oh!

hez2010 Jul 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jakobbotsch Jul 13, 2026

Choose a reason for hiding this comment

Uh oh!

jakobbotsch Jul 13, 2026

Choose a reason for hiding this comment

Uh oh!

hez2010 Jul 13, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Jul 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Holistic Review

Detailed Findings

⚠️ Broadened behavior in impRuntimeLookupToTree — un-spilled helper call in optimized code

✅ callerMethod initialization is complete

✅ Shared-MT vs shared-method distinction

✅ Late-devirtualization context recovery + tests

💡 Style nit in jitinterface.cpp

💡 Open design questions from maintainer review

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

hez2010 commented Jun 24, 2026 •

edited

Loading

hez2010 commented Jul 3, 2026 •

edited

Loading

hez2010 Jul 13, 2026 •

edited

Loading

github-actions Bot commented Jul 19, 2026 •

edited

Loading

⚠️ Broadened behavior in `impRuntimeLookupToTree` — un-spilled helper call in optimized code

✅ `callerMethod` initialization is complete

💡 Style nit in `jitinterface.cpp`